OSDOCS-4744: creating a discrete updating page for mirroring content #55448
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@skopacz1: This pull request references OSDOCS-4744 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
added
the
size/L
|
🤖 Updated build preview is available at: Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/9674 |
openshift-ci
bot
added
size/XL
|
@skopacz1: This pull request references OSDOCS-4744 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Hey @bscott-rh and @bergerhoffer, for this specific PR (i.e. the one that applies to versions 4.11+), I'm technically making some changes to some install/oc-mirror related modules. Could you take a preliminary look to see if these changes look good to you guys? |
|
the ifdefs in the install files look correct to me. |
|
@skopacz1: This pull request references OSDOCS-4744 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@skopacz1: This pull request references OSDOCS-4744 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@skopacz1: This pull request references OSDOCS-4744 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@skopacz1: This pull request references OSDOCS-4744 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
skopacz1
force-pushed
the
4744_main
branch
4 times, most recently
from
ab70b6b to
05266df
Compare
|
@skopacz1: This pull request references OSDOCS-4744 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
bergerhoffer
left a comment
bergerhoffer
left a comment
There was a problem hiding this comment.
Sorry for the delay in getting feedback, still catching up on emails after PTO last week.
Didn't do a full peer review, but I added some feedback in the oc-mirror sections and anything else that jumped out at me. Nice job wrangling this!
installing/disconnected_install/installing-mirroring-disconnected.adoc
Outdated
Show resolved
Hide resolved
updating/updating-restricted-network-cluster/mirroring-image-repository.adoc
Outdated
Show resolved
Hide resolved
updating/updating-restricted-network-cluster/mirroring-image-repository.adoc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I'd make sure that this section is really all you needed to do the oc adm release version. Compared with this section on it: https://55448--docspreview.netlify.app/openshift-enterprise/latest/installing/disconnected_install/installing-mirroring-installation-images.html
Not sure if it's missing anything else from here or not
There was a problem hiding this comment.
Before I started documenting oc mirror as an alternative option, this one section was all the updating documentation had for the explicit mirroring process.
But to your point, wouldn't hurt for me to double check whether any of the sections from your linked page should be included too! Will do
There was a problem hiding this comment.
Okay then that might be it. I was looking at using oc adm release for the installation process - so maybe it is different. Then you're probably right that this might be sufficient as is!
updating/updating-restricted-network-cluster/mirroring-image-repository.adoc
Outdated
Show resolved
Hide resolved
updating/updating-restricted-network-cluster/mirroring-image-repository.adoc
Outdated
Show resolved
Hide resolved
updating/updating-restricted-network-cluster/mirroring-image-repository.adoc
Outdated
Show resolved
Hide resolved
updating/updating-restricted-network-cluster/mirroring-image-repository.adoc
Outdated
Show resolved
Hide resolved
updating/updating-restricted-network-cluster/mirroring-image-repository.adoc
Outdated
Show resolved
Hide resolved
|
@shellyyang1989 PTAL |
|
@zhouying7780 could you please help review? Thanks!! |
openshift-merge-robot
added
the
needs-rebase
openshift-ci
bot
added
the
merge-review-needed
|
/remove-label merge-review-needed |
openshift-ci
bot
removed
the
merge-review-needed
| * After mirroring images for the first time, it is easier to update images in the registry. | ||
| * oc-mirror provides an automated way to mirror the release payload from Quay, and also builds the latest graph-data image for the OpenShift Update Service running in the disconnected environment. |
There was a problem hiding this comment.
@jboxman-rh I added this one line after your peer review, would you mind checking it out to see if it looks good to you?
|
/label merge-review-needed |
openshift-ci
bot
added
the
merge-review-needed
|
/label merge-review-in-progress |
openshift-ci
bot
added
the
merge-review-in-progress
michaelryanpeter
left a comment
michaelryanpeter
left a comment
There was a problem hiding this comment.
This review was only for the 4.11+ content.
Please let me know if you have any questions. Great work here! Outside of the rendering issue, most of these comments are style guide / minimalism nits. Please let me know if you have any questions!
| [id="additional-resources_installing-mirroring-disconnected"] | ||
| == Additional resources | ||
|
|
||
| * See xref:../../updating/updating-restricted-network-cluster/index.adoc#about-restricted-network-updates[About cluster updates in a disconnected environment] for information about updating a cluster in a disconnected environment. |
There was a problem hiding this comment.
| * See xref:../../updating/updating-restricted-network-cluster/index.adoc#about-restricted-network-updates[About cluster updates in a disconnected environment] for information about updating a cluster in a disconnected environment. | |
| * xref:../../updating/updating-restricted-network-cluster/index.adoc#about-restricted-network-updates[About cluster updates in a disconnected environment] |
Nit: While this format is correct for inline links (see ISG quick reference), the repo guidelines specify using only the minimum text necessary for Additional resources sections.
| ifndef::oc-mirror,update-oc-mirror[] | ||
| <1> Specify the path to the folder to store the pull secret in and a name for the JSON file that you create. | ||
| endif::[] | ||
| ifdef::oc-mirror[] | ||
| <1> Specify the path to the folder to store the pull secret in and a name for the JSON file that you create. | ||
|
|
||
| . Save the file either as `~/.docker/config.json` or `$XDG_RUNTIME_DIR/containers/auth.json`. | ||
| endif::[] | ||
| ifdef::update-oc-mirror[] | ||
| <1> Specify the path to the folder to store the pull secret in and a name for the JSON file that you create. | ||
|
|
||
| . Optional: If using the oc-mirror plugin, save the file either as `~/.docker/config.json` or `$XDG_RUNTIME_DIR/containers/auth.json`. | ||
| endif::[] |
There was a problem hiding this comment.
Question for you: It seems like the callout <1> is the same throughout. Does the rendering break if you only conditionalize the sub-sub-step to save the file?
Please feel free to disregard if it is off topic. Something is off here in the conditionals, tho. The callout numbers are getting scrambled. You can see it in the rendered preview:
There was a problem hiding this comment.
As these conditionals start to get pretty embedded, just a friendly reminder that the occasional comment can go a long way to help with the maintainability of the doc in the future.
| == Performing a disconnected environment update | ||
| [id="about-disconnected-updates-mirroring"] | ||
| == Mirroring the {product-title} image repository | ||
| To update your cluster in a disconnected environment, your cluster environment must have access to a mirror registry that contains the necessary images and resources for your targeted update. The following page contains instructions for mirroring images onto a repository in your disconnected cluster: |
There was a problem hiding this comment.
Simple language nit: consider using has instead of contains.
There was a problem hiding this comment.
Optional: Consider breaking the first sentence into two shorter sentences.
| To update your cluster in a disconnected environment, your cluster environment must have access to a mirror registry that contains the necessary images and resources for your targeted update. The following page contains instructions for mirroring images onto a repository in your disconnected cluster: | |
| To update your cluster in a disconnected environment, your cluster environment must have access to a mirror registry. A mirror registry contains the necessary images and resources for your targeted update. The following page contains instructions for mirroring images onto a repository in your disconnected cluster: |
|
|
||
| toc::[] | ||
|
|
||
| You must mirror container images onto a mirror registry before you can update a cluster in a disconnected environment. You can also use this procedure in connected environments to ensure your clusters only use container images that have satisfied your organizational controls on external content. |
There was a problem hiding this comment.
| You must mirror container images onto a mirror registry before you can update a cluster in a disconnected environment. You can also use this procedure in connected environments to ensure your clusters only use container images that have satisfied your organizational controls on external content. | |
| You must mirror container images onto a mirror registry before you can update a cluster in a disconnected environment. You can also use this procedure in connected environments to ensure your clusters run only approved container images that have satisfied your organizational controls for external content. |
The preposition on seems a bit off to me. For sounds a bit better to me. WDYT? Also, PTAL at the ISG's guidance re: only.
There was a problem hiding this comment.
Looks good to me! And thanks for pointing out the guidance about using "only" as an adverb.
|
|
||
| [NOTE] | ||
| ==== | ||
| This registry must be running at all times as long as the cluster is running. |
There was a problem hiding this comment.
See the ISG's guidance on as long as (you need to scroll a little bit). The rest is just a minimalism suggestion.
| This registry must be running at all times as long as the cluster is running. | |
| Your mirror registry must be running while the cluster is running. |
|
|
||
| After your target mirror registry is populated with the initial image set, be sure to update it regularly so that it has the latest content. You can optionally set up a cron job, if possible, so that the mirror registry is updated on a regular basis. | ||
|
|
||
| Ensure that you update your image set configuration to add or remove {product-title} and Operator releases as necessary. Any images that are removed are pruned from the mirror registry. |
There was a problem hiding this comment.
| Ensure that you update your image set configuration to add or remove {product-title} and Operator releases as necessary. Any images that are removed are pruned from the mirror registry. | |
| Update your image set configuration to add or remove {product-title} and Operator releases as necessary. Removed images are pruned from the image registry. |
Minimalism suggestions
There was a problem hiding this comment.
If it's okay with you, I'll implement this but keep the last two words as "mirror registry".
I think that's the terminology that's been consistently used throughout the rest of this page, and I don't know enough to be sure that changing it would be more helpful to users. Let me know if you think I should still change it to "image registry".
There was a problem hiding this comment.
Feel free to use your best judgement :)
There was a problem hiding this comment.
If it isn't otherwise clear, the suggestions are suggestions. I always try to back up actual style guide/mod docs rules with a link to the rule set.
| * Have write access to a container registry in the disconnected environment to push and pull images. The container registry must be compatible with Docker registry API v2. | ||
| * You must have the `oc` command-line interface (CLI) tool installed. | ||
| * For more information on installing Operators, see xref:../../operators/user/olm-installing-operators-in-namespace.adoc#olm-installing-operators-in-namespace[Installing Operators in your namespace]. | ||
| * You must have followed the instructions in xref:../../updating/updating-restricted-network-cluster/mirroring-image-repository.adoc#mirroring-ocp-image-repository[Mirroring the {product-title} image repository] to provision a local container image registry with the necessary container images for your update. |
There was a problem hiding this comment.
| * You must have followed the instructions in xref:../../updating/updating-restricted-network-cluster/mirroring-image-repository.adoc#mirroring-ocp-image-repository[Mirroring the {product-title} image repository] to provision a local container image registry with the necessary container images for your update. | |
| * You must provision a local container image registry with the container images for your update, as described in xref:../../updating/updating-restricted-network-cluster/mirroring-image-repository.adoc#mirroring-ocp-image-repository[Mirroring the {product-title} image repository]. |
| .Additional resources | ||
|
|
||
| * xref:../../installing/disconnected_install/installing-mirroring-disconnected.adoc#installing-mirroring-disconnected[Mirroring images for a disconnected installation using the oc-mirror plugin] | ||
| * For more information on installing Operators, see xref:../../operators/user/olm-installing-operators-in-namespace.adoc#olm-installing-operators-in-namespace[Installing Operators in your namespace]. |
There was a problem hiding this comment.
| * For more information on installing Operators, see xref:../../operators/user/olm-installing-operators-in-namespace.adoc#olm-installing-operators-in-namespace[Installing Operators in your namespace]. | |
| * xref:../../operators/user/olm-installing-operators-in-namespace.adoc#olm-installing-operators-in-namespace[Installing Operators in your namespace]. |
In this case, I think the title of is sufficient.
| * Have write access to a container registry in the disconnected environment to push and pull images. The container registry must be compatible with Docker registry API v2. | ||
| * You must have the `oc` command-line interface (CLI) tool installed. | ||
| * Have access to the cluster as a user with `admin` privileges. | ||
| * You must have followed the instructions in xref:../../updating/updating-restricted-network-cluster/mirroring-image-repository.adoc#mirroring-ocp-image-repository[Mirroring the {product-title} image repository] to provision a local container image registry with the necessary container images for your update. |
There was a problem hiding this comment.
See the note above.
| * If your cluster uses manually maintained credentials, you must ensure that the Cloud Credential Operator (CCO) is in an upgradeable state. For more information, see _Upgrading clusters with manually maintained credentials_ for xref:../../installing/installing_aws/manually-creating-iam.adoc#manually-maintained-credentials-upgrade_manually-creating-iam-aws[AWS], xref:../../installing/installing_azure/manually-creating-iam-azure.adoc#manually-maintained-credentials-upgrade_manually-creating-iam-azure[Azure], or xref:../../installing/installing_gcp/manually-creating-iam-gcp.adoc#manually-maintained-credentials-upgrade_manually-creating-iam-gcp[GCP]. | ||
| //STS is not currently supported in a disconnected environment, but the following bullet can be uncommented when that changes. | ||
| //* If your cluster uses manually maintained credentials with the AWS Security Token Service (STS), obtain a copy of the `ccoctl` utility from the release image being upgraded to and use it to process any updated credentials. For more information, see xref:../../authentication/managing_cloud_provider_credentials/cco-mode-sts.adoc#sts-mode-upgrading[_Upgrading an OpenShift Container Platform cluster configured for manual mode with STS_]. | ||
| * If you run an Operator or you have configured any application with the pod disruption budget, you might experience an interruption during the upgrade process. If `minAvailable` is set to 1 in `PodDisruptionBudget`, the nodes are drained to apply pending machine configs which might block the eviction process. If several nodes are rebooted, all the pods might run on only one node, and the `PodDisruptionBudget` field can prevent the node drain. |
There was a problem hiding this comment.
I know this is out of scope here, but these doesn't feel like a prereq to me. It sounds more like an admonition. WDYT? If you agree, feel free to take it up as a continuous improvement task.
There was a problem hiding this comment.
Totally agree, it's not really telling the user they need to have accomplished anything. I'll add it to my laundry list of minor things to fix in these docs during a later effort.
michaelryanpeter
removed
merge-review-in-progress
|
/cherrypick enterprise-4.13 |
|
/cherrypick enterprise-4.12 |
|
/cherrypick enterprise-4.11 |
|
@michaelryanpeter: #55448 failed to apply on top of branch "enterprise-4.13": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@michaelryanpeter: #55448 failed to apply on top of branch "enterprise-4.12": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@michaelryanpeter: #55448 failed to apply on top of branch "enterprise-4.11": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[enterprise-4.13] OSDOCS#4744: Manual CP of GH #55448
(4.12) Manual CP of GH #55448
[enterprise-4.11] OSDOCS#4744: Manual CP of GH #55448
12 participants
OSDOCS-4744 and OSDOCS-4714
Versions: 4.8+ (NOTE: This specific PR is only for 4.11+)
4.10 PR: #55572
4.9 PR: #55598
4.8 PR: #55604
This PR takes content about preparing mirror hosts and mirroring images out of the two disconnected update pages and moves it onto its own page. For versions 4.11+, the section for using oc-mirror has also been expanded to include full instructions for using the plugin as well as any available reference information.
QE review:
Current/live documentation:
https://docs.openshift.com/container-platform/4.12/updating/updating-restricted-network-cluster/restricted-network-update.html#updating-restricted-network-mirror-host
and
https://docs.openshift.com/container-platform/4.12/updating/updating-restricted-network-cluster/restricted-network-update-osus.html#update-service-mirror-release-osus
Previews:
4.11+: https://55448--docspreview.netlify.app/openshift-enterprise/latest/updating/updating-restricted-network-cluster/mirroring-image-repository.html
4.8-4.10: linked in their respective PRs